Skip to content

Conversation

@karancs06
Copy link
Contributor

optimizing

…entAddButton

- Add microtask wait for Preact component rendering in variant icon test
- Add microtask wait for button generation loops (5 buttons) to complete
- Fixes 2 remaining timeout failures

All tests now passing locally with optimized performance
- Added mock for getFieldVariantStatus() with correct BASE_VARIANT_STATUS structure
- Added mock for FIELD_LOCATION_DATA postMessage event to prevent hanging
- Fixed visualBuilderPostMessage.send() mock to handle all event types properly
- Reverted test timeout back to 30s (root cause was missing mocks, not timeout)
- Tests that were timing out at 30s+ now complete in 5-20s each
- All 813 tests passing locally in ~94 seconds
…eep() calls

- Removed 7 artificial delays (sleep(0) and setTimeout(0))
- Buttons are appended synchronously to DOM, no need to wait
- Test duration: 44s → 9s locally (80% improvement)
- Expected CI improvement: ~7 minutes saved (based on 13x slowdown factor)

Changes:
- Replaced 'await sleep(0)' with direct synchronous DOM queries
- Replaced 'await new Promise(resolve => setTimeout(resolve, 0))' with synchronous queries
- Added clarifying comments that buttons are appended synchronously

All 20 tests still pass. Optimization is safe and maintains test integrity.
@karancs06 karancs06 requested a review from a team as a code owner November 13, 2025 12:37
@github-actions
Copy link

github-actions bot commented Nov 13, 2025

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 71.47% 7974 / 11157
🔵 Statements 71.47% 7974 / 11157
🔵 Functions 74.52% 316 / 424
🔵 Branches 84.91% 1182 / 1392
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/visualBuilder/listeners/mouseHover.ts 64.58% 32.43% 52.38% 64.58% 49-56, 73-74, 114-115, 134-144, 150-151, 158-161, 164-166, 172, 186-187, 198-214, 221-223, 225-268, 276-278, 280-282, 290-298, 301-304, 306-308, 323-327, 370-371, 391-392
Generated in workflow #686 for commit 138535a by the Vitest Coverage Report Action

@karancs06 karancs06 changed the base branch from develop_v3 to develop_v4 November 13, 2025 13:07
@karancs06 karancs06 requested a review from a team as a code owner November 13, 2025 13:07
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just add a single case to check if edit button is visible for any of the field edit modals

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@karancs06 Can we please check this?

vitest.config.ts Outdated
// Reduce retry attempts - with optimized tests, we don't need many retries
retry: 0,
// Timeouts - increased for CI to handle slower async operations
testTimeout: 200000,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

200sec per test suite seems to be too large.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reduced the time

vitest.config.ts Outdated
retry: 0,
// Timeouts - increased for CI to handle slower async operations
testTimeout: 200000,
hookTimeout: 200000,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

200sec for before/after/all/each is too large. Ideally, we increase timeout in cases where DB operation or server are involved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have reduced the time

Comment on lines +54 to +55
testTimeout: 100000,
hookTimeout: 100000,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it are around 30 secs.

Copy link
Contributor Author

@karancs06 karancs06 Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are still 2 to 3 cases that take more time like around 60 to 70 seconds so if we lowered the time they would fail due to timeout so to make sure that each case gets enough time, kept it higher

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, We should break those test cases into multiple suites. Could you please provide list of those modules? Why are they taking 60 secs? Are those due to transform step or before(All|Each) or test suites?

Copy link
Contributor Author

@karancs06 karancs06 Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test cases taking this time are fieldToolbar and fieldLabelWrapper

    {
        "ms": 80701,
        "text": "2025-11-27T11:49:45.3637897Z  ✓ src/visualBuilder/components/__test__/fieldToolbar.test.tsx > FieldToolbarComponent > display variant icon instead of dropdown 80701ms"
    },
    {
        "ms": 63784,
        "text": "2025-11-27T11:53:14.3756962Z  ✓ src/visualBuilder/__test__/hover/fields/group.test.ts > When an element is hovered in visual builder mode > group field (multiple) > should have outline and custom cursor on nested multi line 63784ms"
    },
    {
        "ms": 63290,
        "text": "2025-11-27T11:47:24.8677042Z  ✓ src/visualBuilder/components/__test__/fieldToolbar.test.tsx > FieldToolbarComponent > calls handleMoveInstance with 'next' when move right button is clicked 63290ms"
    },
    {
        "ms": 59848,
        "text": "2025-11-27T11:48:24.6927207Z  ✓ src/visualBuilder/components/__test__/fieldToolbar.test.tsx > FieldToolbarComponent > calls handleDeleteInstance when delete button is clicked 59848ms"
    },
    {
        "ms": 59794,
        "text": "2025-11-27T11:46:21.4552348Z  ✓ src/visualBuilder/components/__test__/fieldToolbar.test.tsx > FieldToolbarComponent > calls handleMoveInstance with 'previous' when move left button is clicked 59794ms"
    },
    {
        "ms": 56692,
        "text": "2025-11-27T11:47:58.2900192Z  ✓ src/visualBuilder/components/__test__/fieldLabelWrapper.test.tsx > FieldLabelWrapperComponent > calls isFieldDisabled with correct arguments 56692ms"
    },
    {
        "ms": 55155,
        "text": "2025-11-27T11:49:28.1310749Z  ✓ src/visualBuilder/__test__/hover/fields/file.test.ts > When an element is hovered in visual builder mode > file field (multiple) > should have outline and custom cursor on individual instances 55155ms"
    },
    {
        "ms": 49105,
        "text": "2025-11-27T11:51:58.3972847Z  ✓ src/visualBuilder/__test__/hover/fields/group.test.ts > When an element is hovered in visual builder mode > group field > should have a outline and custom cursor on the nested single line 49105ms"
    },
    {
        "ms": 45899,
        "text": "2025-11-27T11:47:36.8448884Z  ✓ src/visualBuilder/components/__test__/fieldLabelWrapper.test.tsx > FieldLabelWrapperComponent > renders with correct class when field is disabled 45899ms"
    },
    {
        "ms": 45650,
        "text": "2025-11-27T11:51:06.2954431Z  ✓ src/visualBuilder/components/__test__/fieldToolbar.test.tsx > FieldToolbarComponent > 'Replace button' visibility for multiple file fields > 'replace button' is visible for individual field in multiple file field 45650ms"
    }
]```

@karancs06 karancs06 force-pushed the vitest-3-optimizing branch from 3bb444c to 5c7b09e Compare December 1, 2025 05:37
@karancs06 karancs06 force-pushed the vitest-3-optimizing branch from 8769d9d to 3fc25d1 Compare December 1, 2025 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants